Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ci: Small cleanup of the Github Actions in the PR workflow #465

Merged
merged 4 commits into from
Feb 19, 2025
Merged

ci: Small cleanup of the Github Actions in the PR workflow #465

merged 4 commits into from
Feb 19, 2025

Conversation

martin-g
Copy link
Contributor

@martin-g martin-g commented Feb 17, 2025 •
edited
Loading

  • replace the not maintained actions-rs/toolchain action with dtolnay/rust-toolchain
  • replace actions-rs/cargo with direct usage of cargo
    • replace actions-rs/clippy with direct usage of cargo clippy (it even shows that there are some problems in the code)
  • add build on Linux ARM64. Often Bioconda recipes fail to build on linux-aarch64 because they use a version of rust-htslib that has/had problems for this platform

@martin-g
Copy link
Contributor Author

  • (it even shows that there are some problems in the code)

I could push fix for those if the maintainers of the repo say it is OK to do it in this PR!

@fxwiegand fxwiegand changed the title Small cleanup of the Github Actions in the PR workflow ci: Small cleanup of the Github Actions in the PR workflow Feb 17, 2025
@coveralls
Copy link

coveralls commented Feb 17, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 13408697687

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 33 of 45 (73.33%) changed or added relevant lines in 10 files are covered.
  • 127 unchanged lines in 15 files lost coverage.
  • Overall coverage decreased (-0.5%) to 79.254%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/bam/record_serde.rs 0 1 0.0%
src/bcf/record.rs 6 8 75.0%
src/tbx/mod.rs 0 2 0.0%
src/bam/record.rs 15 22 68.18%
Files with Coverage Reduction New Missed Lines %
src/bam/pileup.rs 1 85.45%
src/utils.rs 1 85.71%
src/bam/ext.rs 3 92.23%
src/bam/index.rs 3 54.55%
src/bcf/index.rs 3 65.63%
src/faidx/mod.rs 3 84.91%
src/bam/buffer.rs 4 51.67%
src/bam/mod.rs 4 82.1%
src/bcf/buffer.rs 5 59.38%
src/bcf/mod.rs 5 76.21%
Totals Coverage Status
Change from base Build 12126773228: -0.5%
Covered Lines: 2506
Relevant Lines: 3162
💛 - Coveralls

@brainstorm brainstorm self-requested a review February 17, 2025 21:29
brainstorm
brainstorm approved these changes Feb 17, 2025
View reviewed changes
Copy link
Member

@brainstorm brainstorm left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very much needed, thanks @martin-g ;)

Please fix the lint errors if you are ok doing so?

@martin-g
Copy link
Contributor Author

Please fix the lint errors if you are ok doing so?

Doing it!

@martin-g
Copy link
Contributor Author

OK! All is green!
Please review when you have time!

@brainstorm
Copy link
Member

brainstorm commented Feb 19, 2025 •
edited
Loading

Thanks much @martin-g, LGTM.

@johanneskoester, I've been approving the workflow runs and checking the changes on almost each commit, any objections from your side?

@brainstorm
Copy link
Member

@martin-g Actually, can you squash your commits a bit? CI tests tend to be a bit verbose on commit history, other than that the changes look great and code looks even better now too ;)

@martin-g
Update GHA actions to their latest versions
2fc7f39 
Keep only the release related actions as is. The action is deprecated
and should be replaces by a new one.

Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@martin-g
Test on Linux ARM64 too
5c01610 
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@martin-g
Fix clippy errors for Rust 1.84.1
0f1b3b8 
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@martin-g
Use dependabot to send PRs for new versions of the used Github Actions
8b27653 
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
@martin-g
Copy link
Contributor Author

can you squash your commits a bit?

Done!

@brainstorm brainstorm merged commit 19902d4 into rust-bio:master Feb 19, 2025
11 of 12 checks passed
@martin-g martin-g deleted the gha-improvements branch February 19, 2025 08:56
@brainstorm brainstorm mentioned this pull request Feb 19, 2025
Closed
@dlaehnemann dlaehnemann mentioned this pull request Feb 24, 2025
Open
@dlaehnemann
Copy link
Member

@martin-g We are implementing the same changes over in the rust-bio repository and I was wondering if it was a deliberate choice to set the runners in rust.yml to ubuntu-22.04 instead of ubuntu-latest? And if so, why?

@martin-g
Copy link
Contributor Author

The OS images (22.04 vs 24.04) come with different set of preinstalled software.
Some package was missing in the ARM64 image but I don't remember which one exactly.
Try with -latest first and use it if it works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brainstorm brainstorm brainstorm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@martin-g @coveralls @brainstorm @dlaehnemann